fix(types): declare id on the filter-builder condition so it stops being stripped - #8432

Refs #8415

Clause-②: yes — this narrows a published accept set. needs:contract-review is hung on this PR and on the card. ⛔ Draft, unqueued, no auto-merge, no self-approval: parking green is the sanctioned resting state.

The defect

FilterBuilderConditionSchema declared a condition as { field, operator, value? } and its body is a plain z.object, which strips undeclared keys. So an author who correctly wrote id had it discarded in silence — the document validated, the row rendered, and from that point the row had no individual identity — every affordance on it is handed undefined, and c.id === conditionId is true for every id-less row, so each affordance acts on all of them at once. Measured under Patch round below.

Measured on this branch's base 0203a29e, through FilterBuilderConditionSchema.safeParse:

That is accepted-and-discarded, not refused — the class #6150 closed for tree-view.title.

Patch round — the mechanism statement was INVERTED, and it shipped

A CONTRACT_REVIEW_TIER reviewer returned FAIL on this PR's own explanation of what breaks when id is stripped. The earlier wording said the row "could never be edited or removed", that removeCondition "deletes every OTHER row" and that updateCondition / changeField "match none". That is the opposite of what the code does, and two of the four surfaces carrying it are published text: the changeset reaches CHANGELOG.md, which is in the package's files[]; the zod comment survives into dist/zod/complex.zod.js; the TSDoc into dist/complex.d.ts.

Re-read independently from packages/components/src/custom/filter-builder.tsx — symbols removeCondition, updateCondition, changeOperator, changeField, and the row key — then simulated on the four helper bodies transcribed verbatim, over three id-less rows plus one crypto.randomUUID() row. That mix is the realistic one: the mirror strips every authored row's id, while rows the user adds in-session are born with one.

Both sides of every comparison are undefined, and undefined === undefined is true, so each helper matches every id-less row rather than none:

⇒ The defect is loss of individual identity — every affordance acts on all the id-less rows en bloc, and a row cannot be edited or removed on its own. It is not "matches none", and it is more severe than the old text claimed, not less.

One correction to the reviewer's dictated wording, because taking it verbatim would have shipped a second wrong mechanism. The instruction said React "keys them all undefined" / that the stripped rows "collide on a duplicate undefined React key". They do not collide: key={condition.id} on an id-less row is key={undefined}, which React reads as no key at all. Measured on React 19.2.8 through react/jsx-runtime, element.key is null for every such row, the list falls back to index reconciliation, and React logs Each child in a list should have a unique "key" prop. The published text now says that, not the duplicate-key version. Everything else in the reviewer's instruction reproduced exactly.

The two additions the reviewer asked for, both measured

The invalid_union path precision. value.conditions.0.id is the logical location — the concatenation of the paths down the arm tree. The issue safeValidateSchema actually reports is a single root invalid_union at path: [] across 13 arms, with the id leaf three nested unions further down, inside arm 8:

invalid_union @ []                                   (13 arms)
  arm 8  -> invalid_union @ ["value"]                 (2 arms)
    arm 1  -> invalid_union @ ["conditions", 0]       (2 arms)
      arm 0  -> invalid_type @ ["id"]
                "Invalid input: expected string, received undefined"

Parsed against FilterBuilderConditionSchema directly, the same refusal is reported flat at path: ["id"]. Both are now stated rather than one deleted, because a consumer that reads issue.path off the document-level result will not find id there.

The compile-time consequence, stated explicitly — it was only implicit before, and #7774 stated its compile-time face (groupField?: never, "refused at compile time"). Measured with tsc --strict against the built dist/complex.d.ts: four positive spellings and two negative controls, exit 0 with every @ts-expect-error consumed.

The message is Property 'id' is missing in type '{ field: string; operator: "equals"; value: string; }' but required in type 'FilterBuilderCondition'. Falsifiability checked: deleting one @ts-expect-error turns the run red (exit 2) with exactly that message.

Comment-only — proved, not asserted

No executable text moved. Method: each .ts file is lexed with the TypeScript parser (ts.createSourceFile), and two independent readings are taken.

1. The leaf-token stream — node.getText() starts at getStart(), i.e. after leading trivia, so comments and whitespace never enter it. Joined and sha256'd. JSDoc subtrees are excluded: getChildren() folds a /** … */ block into the tree as real nodes, so without that exclusion a JSDoc-only edit moves the hash while the token count holds. Measured — and it is why the first version of this prover was wrong.
2. The source with every comment range erased (getLeadingCommentRanges / getTrailingCommentRanges), then lines still holding non-whitespace counted and sha256'd.

Not a regex, and not the raw ts.createScanner: the scanner is context-free and cannot tell a backtick inside a single-quoted string from a template start. Measured on complex.zod.ts, whose describe(...) strings carry backticks, it desynced after 3 comment tokens and emitted a single 2396-character "template" token.

Readings across this round's own commit, 44b668f3 (before) to c7150ca7 (after):

The only reading that moved is complex.zod.ts's comment-range count, 89 → 100 — one long // block split into a bulleted list, which is exactly what a comment-only edit is allowed to move.

Prover falsifiability, four scratch mutations of these same files: a JSDoc-text-only edit and a line-comment-only edit each leave both hashes at the post-edit values, while renaming one identifier (field → fieldX) and changing one string literal ('Field name' → 'Field NAME') each move both hashes.

And the comments really are published — checked in the built dist/ at this head: dist/zod/complex.zod.js carries // helper matches EVERY id-less row rather than none:, and dist/complex.d.ts carries en bloc.) Declaring it is what makes ....

⚠️ One residue that cannot be repaired: the commit message of 44b668f3 still carries the old wording. Amending a pushed commit is forbidden here, and this repo squashes on merge using the PR title and body — so the text that lands on main is this body, not that message.

Gates for this round — head 997280e4

Every exit code captured before any pipe (redirect to a log, capture $?, then read the log).

origin/main had moved (ca394272 → 786bc91e), so it was merged in — never rebased, nothing force-pushed or amended. The merge left all four surfaces byte-identical (git diff c7150ca7 HEAD over them is empty), and main had not touched them.

⛔ No contract change in this round. The id member's shape on both faces, the union, the accept set, the barrel, and every test assertion and fixture are byte-identical — which is precisely what the comment-only table proves. This round is wording only.

Symbols, re-derived on origin/main (no line number copied from #8415 or #7562)

• packages/types/src/zod/complex.zod.ts — FilterBuilderConditionObject, the object the exported z.lazy returns. That is the edit target, not the z.lazy wrapper.
• packages/types/src/complex.ts — FilterBuilderCondition, the TypeScript twin. It omitted id too and moved with the mirror.

⛔ The z.lazy shape is untouched. The docblock directly above the edit point records that this one lazy can be memoised because its body is not recursive, and that seven other z.lazy exports on the same face cannot take that shape. Nothing here reshapes any lazy; zod-lazy-getter-identity-7918.test.ts stays green.

The condition id vs the group id — the trap this card was split out to avoid

.id in packages/components/src/custom/filter-builder.tsx hits 17 (control: a bogus key hits 0). Disambiguated site by site rather than reported as a total:

⇒ 16 condition-side sites, 0 group-side. The card's "four plus the React key" is right about the match sites and the key; the other eleven are call sites feeding them. The two ids take opposite answers and are not unified here: FilterGroupSchema.id stays optional, and a new pin holds them apart.

Three other faces already declared it required — the mirror was the only one that did not:

• the component's own exported FilterBuilderCondition declares id: string;
• addCondition emits id: crypto.randomUUID();
• content/docs/components/complex/filter-builder.mdx publishes id: string; // Condition identifier, with no ?.

Accept-set table, from source, both entry paths

FilterBuilderSchema.value and .defaultValue are each a union of condition and group, so a condition reaches the mirror two ways; conditions also nest inside a group's conditions.

Negative controls — refused before, still refused, so the narrowing did not swallow them: condition with no field; bad operator (with and without id); group spelled operator; group id: 42.

Positive controls — accepted before, still accepted: condition with id; empty group; group without its own id; nested subgroup with id; a document with no value at all; catalog empty-filter-builder and user-filters.

Unchanged and not this PR's business: catalog product-search and with-conditions refuse at base and still refuse, on the operator alias eq / gt / lt — that is #7561, deliberately not folded in. #7562's items 1–3 are untouched: FilterOperatorSchema, FilterFieldSchema and FilterBuilderSchema's own keys are byte-identical.

Corpus census, with a firing control

Instrument: a structural walker over apps/**, examples/**, content/**, packages/** (5027 files) — JSON.parse for JSON, fenced JSON in Markdown/MDX, and the TypeScript parser for object literals in TS/TSX. It reports every object carrying field and operator inside a conditions array.

Authored metadata — the population the re-grade trigger asks about: 7 of 7 conditions carry id, 0 do not. All in examples/schema-catalog/src/schemas/components-complex-filter-builder/ — product-search 3/3, search-interface 2/2, with-conditions 2/2. (empty-filter-builder and user-filters author zero rows.) No document in content/** or apps/** authors a condition; the published doc renders the catalog entry rather than inlining one.

Firing control — a known-positive planted in both directions and then removed: totals moved 59 | 39 with / 20 without → 61 | 40 / 21, and the walker named the planted file and both of its rows (control_positive with an id, control_negative absent). Removal restored the exact baseline and left git status clean. So the zero above is a reading, not a blind walk.

Radius, stated rather than implied: this repository's working tree only. Authorship outside it — downstream applications built on @object-ui/types — is not measurable from here and is not covered by that zero.

Parity: measured, not asserted

The instrument is pnpm --filter @object-ui/types type-check, whose third leg tsc -p tsconfig.test.json compiles the tests the package tsconfig excludes. ⚠️ vitest run zod-mirror-parity.test.ts is a false green here — the ledger is a type map and vitest does not typecheck.

Proved live first, before measuring anything: mirroring ObjectViewSchema.listViews (the key the ledger pins as unmirrored) reddened tsc with

src/__tests__/zod-mirror-parity.test.ts(2333,14): error TS2322: Type '"objectql.zod.ts#ObjectViewSchema"' is not assignable to type 'never'.

while the same mutated tree ran vitest 31/31 green — the false green reproduced. Restore proven by blob hash back to HEAD.

Result on this change: no ledger row and no header figure moves. type-check is green. Mechanistically consistent with the ledger's own entry: complex.zod.ts#FilterBuilderConditionSchema sits in the not-compared half ("declared z.ZodType over any, which exposes no .shape to read"), and FilterBuilderCondition is not one of its paired TypeScript imports. ⇒ zod-mirror-parity.test.ts, held by parked PR #8354, is not touched.

Fixture triage

zod-lazy-getter-identity-7918.test.ts had two condition fixtures without id. Both now carry one, and the negative fixture carries one for a reason that is not cosmetic: without it the row would be refused for the missing key, and an assertion whose subject is FilterOperatorSchema would have stayed green with the operator vocabulary deleted outright. Carrying id isolates the operator.

No other fixture in the mirror's consumer radius needed repair — verified by running the full suites of every package that validates documents through safeValidateSchema.

Reverse verification

Leg 1 — ablate the runtime declaration. Blob 0e69e4d7 → 9a6473bb on disk; anchored counts 1 → 0 for the condition id line and 1 → 1 for the group's optional one, so exactly the member this card added was removed. New pin: 4 failed / 8 passed — the four failures are precisely the narrowing assertions, and the eight that stay green are the negative controls and the source-derived read-site probes, which must not move. Whole types suite under ablation: exactly one file fails, mine (1 failed / 141 passed). Restored: blob equal to HEAD, count back to 1, git diff HEAD empty.

Leg 2 — ablate the TypeScript declaration (id: string to id?: string). Anchored counts 6 → 5 required and 3 → 4 optional. tsc -p tsconfig.test.json exit 1 with both compile-time pins firing:

error TS2344: Type 'false' does not satisfy the constraint 'true'.
error TS2578: Unused '@ts-expect-error' directive.

Restored: blob equal to HEAD, count back to 6, git diff HEAD empty. Both ablation scripts carry an EXIT INT TERM trap restoring from HEAD by absolute path.

Green control re-run after restore: 142 files / 2707 tests pass, type-check green, build green.

Gates — first round, head 44b668f3

Every exit code captured before any pipe. These are the first round's readings, taken before origin/main was merged in; this round's gates, at head 997280e4, are in Patch round above.

Repo-wide eslint . is CI's run, not this PR's. One correction worth recording: an earlier free-hand eslint . --no-inline-config reported 5 errors in packages/types; all five were in files this PR does not touch and were an artifact of --no-inline-config stripping the eslint-disable directives those rule-demonstration pins rely on. objectui's own script is eslint . without that flag, and it exits 0.

Grading input for the PM

The card is priority:p3 with a written-down re-grade trigger: p2 if authored conditions commonly carry id, so the strip is hitting real documents rather than a theoretical author. Measured: 7 of 7 — 100% of authored conditions in this repository carry id, and every one of them was having it discarded. My judgement is that the trigger fires and this is p2. The number is above; the call is the PM's.

Verification notes

• ⛔ Nothing here touches content/docs/releases/, and no branch was force-pushed, rebased or amended after publication.
• ⛔ FilterGroupSchema and its optional id are byte-identical to base.
• The out-of-scope observations found while measuring are in the report, not filed as cards, and none of them is a fix in this PR.

🤖 Generated with Claude Code

Generated by Claude Code

Generated by Claude Code